Skip to content

fix: value-based equality and hashCode for BleCharacteristic - #267

Merged
navidecklabs merged 3 commits into
Navideck:mainfrom
xianjianlf2:fix/ble-characteristic-equality-list
Jul 24, 2026
Merged

fix: value-based equality and hashCode for BleCharacteristic#267
navidecklabs merged 3 commits into
Navideck:mainfrom
xianjianlf2:fix/ble-characteristic-equality-list

Conversation

@xianjianlf2

Copy link
Copy Markdown
Contributor

Problem

BleCharacteristic.operator== compares the properties list with !=, which is List identity comparison in Dart, and hashCode uses properties.hashCode, which is also identity-based. As a result, two characteristics with identical content are never equal unless they share the exact same list instance:

final a = BleCharacteristic("180A", [CharacteristicProperty.read]);
final b = BleCharacteristic("180A", [CharacteristicProperty.read]);
print(a == b); // false — but they are logically equal

This breaks the ==/hashCode contract and makes BleCharacteristic unreliable as a Set element or Map key (e.g. deduplicating characteristics across discovery runs).

Fix

  • operator== now compares properties with listEquals.
  • hashCode now uses Object.hashAll(properties).

This matches the pattern already used by ManufacturerData in this package.

Testing

  • Added test/ble_characteristic_equality_test.dart covering: equal content ⇒ equal + same hash, differing properties ⇒ unequal, differing uuid ⇒ unequal, and Set/Map key usage.
  • Ran flutter test test/ble_characteristic_equality_test.dart locally (Flutter 3.44.1 / Dart 3.12.1): 4 tests, all passing.

Changelog

Added an entry under the unreleased 2.1.1 section in CHANGELOG.md.

BleCharacteristic.operator== compared the `properties` list with `!=`,
which is List identity comparison, and hashCode used `properties.hashCode`
(also identity based). As a result two characteristics with equal content
but distinct list instances were never equal and produced different hash
codes, breaking the equality contract and Set/Map lookups.

Use `listEquals` for comparison and `Object.hashAll` for hashing, matching
the pattern already used in ManufacturerData.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes the equality and hashCode implementation of BleCharacteristic to compare properties by value using listEquals and Object.hashAll, and adds corresponding unit tests. The reviewer suggested using Object.hash instead of the bitwise XOR (^) operator to combine hash codes to prevent potential hash collisions.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread lib/src/models/ble_service.dart Outdated
@xianjianlf2

Copy link
Copy Markdown
Contributor Author

Addressed the hash-combining review comment in 29e19ff by replacing the XOR combination with Object.hash(uuid, Object.hashAll(properties), metaData).\n\nI attempted the targeted Flutter test locally, but this checkout's configured FVM Flutter is Dart 3.5.3 while the package now requires Dart ^3.12.0, so dependency resolution cannot run in this local environment.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes BleCharacteristic’s equality/hash semantics so two instances with identical UUID + properties compare equal and can be reliably used as Set elements / Map keys, and adds a regression test plus a changelog entry.

Changes:

  • Update BleCharacteristic.operator== to compare properties by value (listEquals) and update hashCode to hash list contents (Object.hashAll).
  • Add unit tests validating equality + hashCode behavior.
  • Add a changelog entry under 2.1.1.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
test/ble_characteristic_equality_test.dart Adds regression tests for value-based equality/hashCode behavior.
lib/src/models/ble_service.dart Implements value-based list equality + content-based hashing for properties.
CHANGELOG.md Documents the fix in the unreleased 2.1.1 section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/src/models/ble_service.dart
Comment thread test/ble_characteristic_equality_test.dart
Co-authored-by: Cursor <cursoragent@cursor.com>
@navidecklabs
navidecklabs merged commit 975f37b into Navideck:main Jul 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants